Skip to content

Add history.clearContext and Tool.isTerminal across all SDKs - #2129

Open
examon wants to merge 5 commits into
mainfrom
clearcontext-rpc
Open

Add history.clearContext and Tool.isTerminal across all SDKs#2129
examon wants to merge 5 commits into
mainfrom
clearcontext-rpc

Conversation

@examon

@examon examon commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Overview

What

Adds two things to every SDK language surface:

  1. history.clearContext on the generated session RPC client — clears the conversation (keeping system and developer messages) and seeds the fresh context window with a required first user message. The runtime rejects the call unless it is made from inside a tool handler with a tool call in flight: that is the only state in which the clear can drop the tool results its wipe orphans, so a clear from a hook, a slash-command handler or a background timer is refused rather than corrupting the window. Also picks up the new session.context_cleared event.
  2. isTerminal on the tool definition — lets a tool declare that a successful call ends the agent turn instead of the result being fed back to the model for another round. A failed call leaves the loop running so the model can read the error and retry.
const session = await joinSession({
    tools: [{
        name: "clear_context",
        isTerminal: true,
        defer: "never",
        parameters: {
            type: "object",
            properties: { prompt: { type: "string" } },
            required: ["prompt"],
        },
        handler: async ({ prompt }) => {
            const { messagesCleared } = await session.rpc.history.clearContext({ prompt });
            return { textResultForLlm: `Cleared ${messagesCleared} message(s).`, resultType: "success" };
        },
    }],
});

Why

Together these let a context-clearing — or handoff, or any turn-ending — tool be implemented by an SDK consumer or extension, instead of requiring one built into the runtime.

Without isTerminal such a tool can only approximate turn-ending by returning a rejected result, which halts the loop but is semantically wrong and surfaces as a user rejection. Without clearContext the capability has no API surface at all.

Per-language changes

Generated RPC/event types are regenerated for all six languages. isTerminal is hand-authored per language, matching how overridesBuiltInTool / skipPermission / defer are already carried:

Language Tool flag Serialization
Node.js Tool.isTerminal, defineTool config both createSession / resumeSession sites in client.ts
Go Tool.IsTerminal json:"isTerminal,omitempty"
Python Tool.is_terminal, define_tool overloads both client serialization sites
Rust Tool::is_terminal skipped when false
Java ToolDefinition.isTerminal record component @JsonProperty("isTerminal")
.NET CopilotToolOptions.IsTerminal, is_terminal additional-property key wire ToolDefinition

Coexistence with Tool.metadata. metadata landed on main while this branch was open, and it touched exactly the same tool-option surfaces. The branch is rebased on top of it and the two are independent, additive options everywhere: Tool.metadata + Tool.isTerminal (Node), metadata + is_terminal (Python), Metadata + IsTerminal (.NET), and so on.

Java source compatibility. ToolDefinition is a record, so adding a component changes the canonical constructor. The canonical form is now nine components (…, defer, metadata, isTerminal), with two convenience constructors that delegate for the older shapes:

  • seven arguments (…, defer) → metadata = null, isTerminal = null
  • eight arguments (…, defer, metadata) → isTerminal = null

so existing call sites — including annotation-processor output — keep compiling unchanged. Tests pin both.

Every fluent copy method (overridesBuiltInTool, skipPermission, defer, metadata) threads isTerminal through, so it is not silently dropped when another option is set afterwards, and a matching isTerminal(boolean) copy method is added for tools built via the from(...) factories.

resumeSession is the path extensions join on via joinSession, so it matters that both serialization sites carry the flag, not just session creation.

Tests

Language Test
Node.js client.test.ts - isTerminal forwarded on both session.create and session.resume, and omitted when unset
Go TestIsTerminal - camelCase wire name when set, omitted when false
Python test_client.py - isTerminal forwarded on both session.create and session.resume, and omitted at its default
Rust is_terminal_tests - the same two cases via serde_json, plus a guard that the hand-written Debug impl reports the field
Java ToolDefinitionIsTerminalTest - both cases plus the older-arity constructor compatibility guard
.NET CopilotToolTests - is_terminal additional property set when requested, omitted otherwise

Tool is the one type here with a hand-written Debug impl in Rust rather than a derived one, so a new field is only reported if it is added there by hand. The Rust test asserts that, and fails if the impl drifts.

Validation

All six SDK test matrices pass in CI across Linux, macOS and Windows, as do every Validate * and CodeQL Analyze * job. The .NET tests noted as uncompiled in an earlier revision of this description have since been built and run by CI on all three platforms.

Locally, re-run after the rebase onto 1.0.78: tsc --noEmit, go build/vet/test, cargo test --lib (213 passing), mvn test (95 test classes, 0 failures, including ToolDefinitionIsTerminalTest), ruff check/format, dotnet build, and the two new Vitest isTerminal cases.

Node.js test/e2e/* and Go internal/e2e need a Copilot CLI binary that is unavailable locally; they are covered by CI.

Codegen (resolved)

The Codegen Check failure is resolved. Sequence:

  • github/copilot-agent-runtime#14002 merged 2026-08-03 at 19:01Z.
  • @github/copilot@1.0.78, published 2026-08-03 at 23:30Z, is the first release shipping the schemas: schemas/api.schema.json defines clearContext with "rpcMethod": "session.history.clearContext", and schemas/session-events.schema.json defines context_cleared.
  • The repo pin moved to 1.0.78 on main, and this branch is now rebased on top of that.

Because main regenerates from the real schema, the generated bindings now come from main and this branch no longer carries a single generated file. The diff is purely the hand-written SDK surface across the six languages. Verified locally after the rebase: cd scripts/codegen && npm run generate and cd java && mvn generate-sources -Pcodegen each produce zero drift.

This also closes the Java gap raised in review. While the pinned CLI lacked the schemas, the java-codegen-check workflow auto-committed a regeneration that stripped SessionContextClearedEvent, SessionHistoryClearContextParams/Result and the SessionHistoryApi method. That strip commit is obsolete and was dropped during the rebase, and main's regenerated Java bindings now supply all of it, so Java is at parity with the other SDKs.

The generated clearContext docs now reflect the tightened contract from the shipped schema: the seed prompt is required rather than optional.

One consequence worth flagging for review: Codegen Check is path-filtered on the generated directories, so now that this branch touches none of them the workflow no longer triggers at all. Its absence from the checks list is the expected outcome, not a skipped or disabled check.

Notes

  • Purely additive; every new field is optional and absent means today's behavior.
  • No generated files remain in this diff. The bindings come from main's regeneration against @github/copilot@1.0.78.

Depends on github/copilot-agent-runtime#14002 (merged 2026-08-03), which adds the runtime surface.

Copilot AI balanced review requested due to automatic review settings July 29, 2026 17:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Node SDK support for clearing session context and terminal tools.

Changes:

  • Adds Tool.isTerminal and forwards it during create/resume.
  • Adds generated history.clearContext RPC types and client method.
Show a summary per file
File Description
nodejs/src/types.ts Exposes terminal-tool configuration.
nodejs/src/client.ts Serializes isTerminal in session requests.
nodejs/src/generated/rpc.ts Adds context-clearing RPC support.

Review details

  • Files reviewed: 2/3 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread nodejs/src/types.ts
Copilot AI review requested due to automatic review settings July 29, 2026 19:47
@examon
examon force-pushed the clearcontext-rpc branch from 86d300b to 06dc43a Compare July 29, 2026 19:47
@examon examon changed the title Add history.clearContext and Tool.isTerminal Add history.clearContext and Tool.isTerminal across all SDKs Jul 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (4)
  • go/rpc/zrpc.go: Generated file
  • go/rpc/zsession_encoding.go: Generated file
  • go/rpc/zsession_events.go: Generated file
  • go/zsession_events.go: Generated file
Comments suppressed due to low confidence (5)

java/src/main/java/com/github/copilot/rpc/ToolDefinition.java:87

  • The ergonomic annotation path cannot configure this new flag. @CopilotTool currently exposes override, permission, and defer options, and CopilotToolProcessor forwards each one, but neither has an isTerminal member. Users defining tools through the documented annotation API therefore cannot declare terminal tools; add the annotation property and processor wiring (with processor coverage).
        @JsonProperty("skipPermission") Boolean skipPermission, @JsonProperty("defer") ToolDefer defer,
        @JsonProperty("isTerminal") Boolean isTerminal) {

rust/src/types.rs:352

  • Tool is #[non_exhaustive] and its docs direct consumers to the fluent builder, but this new option has no with_is_terminal method. Every adjacent runtime hint does (with_overrides_built_in_tool, with_skip_permission, and with_defer at types.rs:455-477), and the custom Debug implementation at types.rs:496-511 also omits this field. Please integrate is_terminal into both APIs so consumers do not have to switch to post-construction mutation and diagnostics show the configured value.
    #[serde(default, skip_serializing_if = "is_false")]
    pub is_terminal: bool,

java/src/main/java/com/github/copilot/rpc/ToolDefinition.java:73

  • Java currently receives only the terminal-tool half of this cross-SDK feature. Unlike Node, Python, Go, Rust, and .NET in this diff, SessionHistoryApi still has no clearContext method and SessionEvent has no typed session.context_cleared event, so Java consumers cannot implement the context-clearing example. Regenerate the Java RPC/event surface from the updated runtime schemas as well.

This issue also appears on line 86 of the same file.

 * @param isTerminal
 *            when {@code true}, a successful call to this tool ends the agent
 *            turn: the runtime's tool phase halts instead of feeding the result
 *            back to the model for another round; {@code null} or {@code false}
 *            leaves the turn running

python/copilot/tools.py:129

  • The public define_tool signature now accepts is_terminal, but its Args section documents every option except this one. Add its turn-ending semantics and default to the docstring so decorator and function-style users can discover the option through help() and generated API documentation.
    is_terminal: bool = False,

go/types.go:1185

  • The PR description says this is “Two small additions to the Node SDK,” lists only three Node files, and reports only npm validation, but the actual change adds public/generated surfaces across Rust, Python, Java, Go, and .NET as well. Please either scope the diff back to Node or update the description and validation evidence for every affected SDK so the review and release impact are accurate.
	// IsTerminal reports that a successful call to this tool ends the agent
	// turn: the runtime halts instead of feeding the result back to the model
	// for another round. A failed call leaves the loop running so the model can
	// read the error and retry.
	IsTerminal bool `json:"isTerminal,omitempty"`
  • Files reviewed: 11/24 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

Copilot AI review requested due to automatic review settings July 29, 2026 20:09
@examon
examon force-pushed the clearcontext-rpc branch from 06dc43a to 11426a7 Compare July 29, 2026 20:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (4)
  • go/rpc/zrpc.go: Generated file
  • go/rpc/zsession_encoding.go: Generated file
  • go/rpc/zsession_events.go: Generated file
  • go/zsession_events.go: Generated file
Comments suppressed due to low confidence (3)

nodejs/src/client.ts:1394

  • The new create/resume forwarding is untested even though nodejs/test/client.test.ts has paired request-payload tests for the adjacent overridesBuiltInTool and defer flags. Add equivalent create and resume assertions for isTerminal so a missing serialization path cannot regress unnoticed.
                    isTerminal: tool.isTerminal,

python/copilot/client.py:1877

  • There is no test covering this new wire serialization, while python/test_client.py already verifies both create and resume forwarding for the neighboring override/defer flags. Add corresponding is_terminal=True tests for both paths (and omission at the default) to protect the advertised behavior.
                if tool.is_terminal:
                    definition["isTerminal"] = True

java/src/main/java/com/github/copilot/rpc/ToolDefinition.java:87

  • The Java portion still does not expose history.clearContext or the typed session.context_cleared event promised for every SDK. SessionHistoryApi currently ends with summarizeForHandoff, and SessionEvent has no context-cleared subtype, so Java consumers cannot use either new generated surface. Please regenerate and commit the Java RPC request/result/API and session-event classes as well.
        @JsonProperty("skipPermission") Boolean skipPermission, @JsonProperty("defer") ToolDefer defer,
        @JsonProperty("isTerminal") Boolean isTerminal) {
  • Files reviewed: 12/25 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

@examon
examon force-pushed the clearcontext-rpc branch from 11426a7 to 0b328ff Compare July 29, 2026 20:36
Copilot AI review requested due to automatic review settings July 29, 2026 20:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (4)
  • go/rpc/zrpc.go: Generated file
  • go/rpc/zsession_encoding.go: Generated file
  • go/rpc/zsession_events.go: Generated file
  • go/zsession_events.go: Generated file
Comments suppressed due to low confidence (2)

java/src/main/java/com/github/copilot/rpc/ToolDefinition.java:87

  • Java's annotated-tool surface cannot set this new flag. @CopilotTool exposes the other tool flags (overridesBuiltInTool, skipPermission, and defer), but the annotation and processor still emit only the seven-argument constructor, so every annotation-defined tool gets isTerminal = null. Please add an isTerminal annotation member and carry it through CopilotToolProcessor, with processor coverage, so the feature is available through the SDK's ergonomic tool API rather than only direct record construction.
        @JsonProperty("skipPermission") Boolean skipPermission, @JsonProperty("defer") ToolDefer defer,
        @JsonProperty("isTerminal") Boolean isTerminal) {

python/copilot/tools.py:129

  • is_terminal is a new public argument but is missing from the function's Args documentation, while every other option is documented there. Add its successful-call/failed-call behavior to the docstring so users can discover the flag without reading the dataclass source.
    is_terminal: bool = False,
  • Files reviewed: 12/30 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

@examon
examon marked this pull request as ready for review July 29, 2026 20:46
@examon
examon requested a review from a team as a code owner July 29, 2026 20:46
@examon
examon force-pushed the clearcontext-rpc branch from 0b328ff to 55cebf9 Compare July 29, 2026 21:21
Copilot AI review requested due to automatic review settings July 29, 2026 21:21
@github-actions github-actions Bot added the dependencies Pull requests that update a dependency file label Jul 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (4)
  • go/rpc/zrpc.go: Generated file
  • go/rpc/zsession_encoding.go: Generated file
  • go/rpc/zsession_events.go: Generated file
  • go/zsession_events.go: Generated file
Comments suppressed due to low confidence (3)

java/src/main/java/com/github/copilot/rpc/ToolDefinition.java:95

  • The Java surface still lacks the other half of this PR: SessionHistoryApi has no clearContext method/request/result types, and there is no typed session.context_cleared event. As a result, Java consumers cannot use the capability that the PR promises for every SDK. Please regenerate and commit the Java RPC and event sources from the updated schema.
        @JsonProperty("metadata") Map<String, Object> metadata, @JsonProperty("isTerminal") Boolean isTerminal) {

python/copilot/client.py:2250

  • Please add a unit test that exercises is_terminal through define_tool and verifies both session.create and session.resume payloads, including omission when false. The adjacent metadata test covers this same serialization path, but these new branches and helper propagation currently have no Python coverage.
                if tool.is_terminal:
                    definition["isTerminal"] = True

dotnet/src/Client.cs:2790

  • The added tests only verify the intermediate AIFunction.AdditionalProperties; they do not exercise this conversion or the serialized session request. Please add coverage asserting that isTerminal reaches both session.create and session.resume payloads (and is omitted by default), otherwise the actual wire path can regress while the current tests still pass.
            var isTerminal = function.AdditionalProperties.TryGetValue(CopilotTool.IsTerminalKey, out var terminalVal) && terminalVal is true;
            return new ToolDefinition(function.Name, function.Description, function.JsonSchema,
                overrides ? true : null,
                skipPerm ? true : null,
                defer,
                metadata,
                isTerminal ? true : null);
  • Files reviewed: 13/26 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@examon
examon force-pushed the clearcontext-rpc branch from fccbbc5 to e25257e Compare July 30, 2026 04:51
Copilot AI review requested due to automatic review settings July 30, 2026 04:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (4)
  • go/rpc/zrpc.go: Generated file
  • go/rpc/zsession_encoding.go: Generated file
  • go/rpc/zsession_events.go: Generated file
  • go/zsession_events.go: Generated file
Comments suppressed due to low confidence (2)

dotnet/src/Client.cs:2790

  • The added tests stop at the AIFunction.AdditionalProperties bag, so they do not exercise this conversion or the serialized session.create/session.resume payload. This new bridge is where is_terminal becomes wire-level isTerminal; add a public-API client test using the existing fake server pattern in ClientSessionLifetimeTests that asserts both requests contain tools[0].isTerminal == true and that the property is omitted when unset.
            var isTerminal = function.AdditionalProperties.TryGetValue(CopilotTool.IsTerminalKey, out var terminalVal) && terminalVal is true;
            return new ToolDefinition(function.Name, function.Description, function.JsonSchema,
                overrides ? true : null,
                skipPerm ? true : null,
                defer,
                metadata,
                isTerminal ? true : null);

python/copilot/client.py:2250

  • Add Python regression coverage for this new wire mapping. python/test_client.py:574-609 already verifies the analogous metadata option on both create and resume, but no test currently exercises is_terminal; a future edit could silently drop either branch or emit the wrong camel-case key. Cover True on both paths and omission for the default False.
                if tool.is_terminal:
                    definition["isTerminal"] = True
  • Files reviewed: 13/26 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread java/src/main/java/com/github/copilot/rpc/ToolDefinition.java
@github-actions

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (4)
  • go/rpc/zrpc.go: Generated file
  • go/rpc/zsession_encoding.go: Generated file
  • go/rpc/zsession_events.go: Generated file
  • go/zsession_events.go: Generated file
Suppressed comments (3)

python/copilot/client.py:2250

  • This new wire mapping has no regression coverage in Python. python/test_client.py:574-609 already verifies the equivalent metadata option on both session.create and session.resume, including omission when unset; add matching is_terminal coverage so either duplicated serialization branch cannot regress.
                if tool.is_terminal:
                    definition["isTerminal"] = True

dotnet/src/Client.cs:2790

  • The added tests only verify that CopilotToolOptions writes the internal is_terminal additional-property key; they do not exercise this conversion into the wire ToolDefinition. Add a public-API fake-server test that inspects both session.create and session.resume payloads (and the unset case), since this mapping is the step that actually emits isTerminal to the runtime.
            var isTerminal = function.AdditionalProperties.TryGetValue(CopilotTool.IsTerminalKey, out var terminalVal) && terminalVal is true;
            return new ToolDefinition(function.Name, function.Description, function.JsonSchema,
                overrides ? true : null,
                skipPerm ? true : null,
                defer,
                metadata,
                isTerminal ? true : null);

java/src/main/java/com/github/copilot/rpc/ToolDefinition.java:398

  • This method is introduced by this PR, but its Javadoc says it has existed since 1.0.7. That gives consumers incorrect version-availability information; set @since to the first Java SDK release that will actually contain isTerminal.
     * @since 1.0.7
  • Files reviewed: 13/26 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

This comment has been minimized.

Copilot AI review requested due to automatic review settings August 2, 2026 16:48
@examon
examon force-pushed the clearcontext-rpc branch from 76eae0c to 3b9b97a Compare August 2, 2026 16:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (4)
  • go/rpc/zrpc.go: Generated file
  • go/rpc/zsession_encoding.go: Generated file
  • go/rpc/zsession_events.go: Generated file
  • go/zsession_events.go: Generated file
Suppressed comments (1)

python/copilot/client.py:2275

  • Add a Python regression test for this forwarding path. python/test_client.py:614-649 already verifies the analogous metadata option on both session.create and session.resume, but there is no test for is_terminal anywhere under Python; cover both paths and the default omission so either serialization site cannot regress independently.
                if tool.is_terminal:
                    definition["isTerminal"] = True
  • Files reviewed: 13/26 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

This comment has been minimized.

examon and others added 3 commits August 4, 2026 02:29
Regenerates the RPC clients for the new `session.history.clearContext` method
and the `session.context_cleared` event, and adds a hand-authored `isTerminal`
tool flag to every language surface.

`isTerminal` lets a tool declare that a successful call ends the agent turn:
the runtime's tool phase halts instead of feeding the result back to the model
for another round. A failed call leaves the loop running so the model can read
the error and retry. Without it a turn-ending tool can only approximate the
behavior by returning a rejected result, which halts the loop but is
semantically wrong.

Per language:
- Node.js: `Tool.isTerminal`, `defineTool` config, both session-config
  serialization sites.
- Go: `Tool.IsTerminal` with `json:"isTerminal,omitempty"`.
- Python: `Tool.is_terminal`, `define_tool` overloads, both client
  serialization sites.
- Rust: `Tool::is_terminal`, skipped when false.
- Java: `ToolDefinition.isTerminal` as a record component, plus a
  seven-argument convenience constructor so existing call sites keep compiling.
- .NET: `CopilotToolOptions.IsTerminal`, the `is_terminal` additional-property
  key, and the wire `ToolDefinition`.

Adds serialization tests in Go, Rust and Java covering both the camelCase wire
name and omission when unset; the Java test also pins the seven-argument
constructor so the record change stays source-compatible.
…ests

Resolves the rebase onto main where both sides added an eighth tool
option: main added metadata and this branch added isTerminal.

- Java ToolDefinition now carries metadata and isTerminal as separate
  record components, keeps the seven- and eight-argument convenience
  constructors, and threads isTerminal through every fluent copy method
  so it is no longer dropped by .metadata()/.defer()/etc.
- Adds ToolDefinition.isTerminal(boolean) so lambda-defined tools can
  set it, matching the other flags.
- Adds the missing Node regression tests asserting isTerminal is
  forwarded on both session.create and session.resume, and omitted when
  unset.
- Applies the repo rust formatter to the new is_terminal test.
Mirrors github/copilot-agent-runtime#14002 after review:

- `HistoryClearContextRequest.prompt` is now required. A cleared window
  holding only system and developer messages is not a conversation a model
  can answer, so every clear seeds the window it creates.
- `HistoryClearContextResult` loses the `cleared` discriminator. The RPC now
  rejects the cases it was meant to describe - a remote session, or a call
  made while no tool call is in flight - so there is one error channel
  instead of a success flag plus an error channel.
- `ContextClearedData.prependMessages` is gone. It had no producer, and it
  was a permanent commitment on a durable event for a code path nothing
  exercised.

Regenerated with `scripts/codegen`; only the clear-context hunks are taken,
so unrelated schema drift stays out of this PR.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 4, 2026 02:45
@examon
examon force-pushed the clearcontext-rpc branch from 3b9b97a to 012f981 Compare August 4, 2026 02:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (2)

python/copilot/client.py:2281

  • The Python implementation has no regression coverage for this wire flag: existing tests exercise analogous tool metadata on both session.create and session.resume (python/test_client.py:756-794), but no test mentions is_terminal. Add assertions that True becomes isTerminal on both paths and that the default is omitted; this also guards the duplicated serialization branches from drifting.
                if tool.is_terminal:
                    definition["isTerminal"] = True

rust/src/types.rs:354

  • Tool has a hand-written Debug implementation that lists every other serializable field, but this new field is not included (rust/src/types.rs:523-539). As a result, debugging/logging a terminal tool reports the same state as a non-terminal tool. Add an is_terminal entry to that implementation.
    /// When `true`, a successful call to this tool ends the agent turn: the
    /// runtime's tool phase halts instead of feeding the result back to the
    /// model for another round. A failed call leaves the loop running so the
    /// model can read the error and retry.
    #[serde(default, skip_serializing_if = "is_false")]
    pub is_terminal: bool,
  • Files reviewed: 13/13 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions github-actions Bot mentioned this pull request Aug 4, 2026
@examon
examon marked this pull request as ready for review August 5, 2026 09:20
@examon

examon commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

The runtime dependency has landed, so this is out of draft and ready for review.

github/copilot-agent-runtime#14002 merged 2026-08-03 19:01Z, @github/copilot@1.0.78 shipped the schemas at 23:30Z, and main regenerated against it. After the rebase this branch carries no generated files at all - the diff is purely the hand-written isTerminal surface across the six languages, which main still does not have.

Verified against main and this branch: HistoryClearContextRequest.prompt is required, HistoryClearContextResult is { messagesCleared } with no cleared discriminator, ContextClearedData.prependMessages is gone, and the method doc carries the tool-handler precondition. All six languages still need the hand-written half.

The description's opening summary still described prompt as optional and omitted the tool-handler precondition; both are corrected, and the example now shows required: ["prompt"].

Worth keeping in mind for review: isTerminal is load-bearing, not cosmetic. Driving a terminal clear_context extension through the real CLI against an SDK build that drops the flag, the agentic loop continues past the terminal tool and burns an extra model call answering a system reminder before the seeded turn runs. With the flag forwarded the event log is exactly one clear, one terminal turn, one seed turn.

The automated reviewer raised two suppressed findings against the new
Tool.isTerminal field. Both were accurate.

Rust: Tool has a hand-written Debug impl that enumerates every other
serializable field, so is_terminal was silently missing from its output
and a terminal tool debug-printed identically to a plain one. Add the
field in declaration order, plus a test that fails if the hand-written
impl drifts again.

Python: dotnet, go, java, nodejs and rust all assert that isTerminal
reaches the wire on both the session.create and session.resume paths and
is omitted at its default. Python was the only SDK without that
coverage. Add the test, mirroring the adjacent tool-metadata test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 5, 2026 09:42
@examon

examon commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Closed the two suppressed findings from the 2026-08-04 automated review in 5000d02. Both were accurate.

Rust — is_terminal missing from the hand-written Debug impl. Tool does not derive Debug; it has a hand-written impl (rust/src/types.rs:523) that enumerates every other serializable field, so the new flag was silently absent and a terminal tool debug-printed identically to a plain one. Added the field in declaration order, between skip_permission and defer.

Rather than just add the line, I added a test for it, because the underlying hazard is that any future field on Tool hits this same trap:

/// `Tool` has a hand-written `Debug` impl, so a new field is only reported
/// if it is added there by hand. Guard against that drift.
#[test]
fn is_terminal_appears_in_debug_output() { ... }

Verified it is load-bearing: removing the .field("is_terminal", ...) line makes it fail at assert!(format!("{terminal:?}").contains("is_terminal: true")), and restoring it makes it pass.

Python — no wire coverage for the flag. The reviewer was right that Python was uncovered, and it was the only SDK in that state. Coverage on the branch before this commit:

SDK isTerminal wire test
dotnet dotnet/test/Unit/CopilotToolTests.cs
go go/client_test.go:3482
java java/.../ToolDefinitionIsTerminalTest.java
nodejs nodejs/test/client.test.ts:717
rust rust/src/types.rsmod is_terminal_tests
python (none)

Added test_create_and_resume_session_forward_tool_is_terminal, mirroring the adjacent ..._forward_tool_metadata test: asserts True becomes isTerminal on both session.create and session.resume, and that it is omitted at its default.

client.py serializes the flag in two independent branches (create_session at 2280, resume_session at 2985), which is exactly the duplication the reviewer flagged as drift-prone, so I checked the test pins both. Deleting either branch fails the test at the matching assertion:

  • remove the create_session branch -> fails at assert create_tools[0]["isTerminal"] is True
  • remove the resume_session branch -> fails at assert captured["session.resume"]["tools"][0]["isTerminal"] is True

Verification: cargo test --lib 213 passed, cargo clippy --all-targets -D warnings clean, cargo fmt --check clean, pytest test_client.py 110 passed, ruff check / ruff format --check clean.

No production behavior changes - one Debug field and two tests.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

java/src/main/java/com/github/copilot/rpc/ToolDefinition.java:398

  • This API is introduced by the current change, so @since 1.0.7 incorrectly claims it existed in an already released version. The nearby newly added Java APIs use 1.0.11; mark this method with that release as well so generated API documentation does not misstate availability.
     * @since 1.0.7
  • Files reviewed: 14/14 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

This comment has been minimized.

ToolDefinition.isTerminal(boolean) is introduced by this PR but was
tagged @SInCE 1.0.7, a version released long ago in which the method did
not exist, so the generated javadoc would misstate its availability.

1.0.11 is the current unreleased version: the pom is at
1.0.10-preview.3-SNAPSHOT, and the eight java/src/main files carrying
@SInCE 1.0.11 include ToolDefinition.createOverride in this same file.

This was the only @SInCE tag the PR added.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 5, 2026 10:15
@examon

examon commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Also fixed the @since finding from the latest automated review in 3ba3ff2. It was correct.

ToolDefinition.isTerminal(boolean) is introduced by this PR but was tagged @since 1.0.7 — a version released long ago, in which the method did not exist — so the generated javadoc would have misstated its availability.

1.0.11 is the right value: java/pom.xml is at 1.0.10-preview.3-SNAPSHOT, and the eight java/src/main files already carrying @since 1.0.11 include ToolDefinition.createOverride in this very file.

Two things I checked while I was in there:

  • This was the only @since the PR added. git diff origin/main...HEAD | grep '^+.*@since' returns exactly one line, so there is no second instance of the same mistake anywhere in the change.
  • The new convenience constructor deliberately has no @since. The ToolDefinition(..., ToolDefer, Map<String, Object>) overload added here omits the tag, which matches the pre-existing 7-arg overload directly above it — neither constructor carries one. So that omission is the existing convention, not a second miss.

mvn spotless:check passes.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 14/14 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅

This PR adds Tool.isTerminal and history.clearContext across all six SDK implementations. I reviewed the authoritative PR diff and cross-referenced each SDK.

isTerminal — all six SDKs covered

SDK Field Wire key Omit-when-unset
Node.js Tool.isTerminal?: boolean isTerminal ✅ (undefined)
Python Tool.is_terminal: bool = False isTerminal ✅ (if tool.is_terminal)
Go Tool.IsTerminal bool isTerminal,omitempty
.NET CopilotToolOptions.IsTerminal bool is_terminal key → wire ToolDefinition.IsTerminal
Java ToolDefinition.isTerminal record component @JsonProperty("isTerminal") ✅ (null = omitted)
Rust Tool.is_terminal: bool (rename_all = "camelCase") isTerminal ✅ (skip_serializing_if = "is_false")

All serialization wire keys land as isTerminal (camelCase) consistently.

history.clearContext — all six SDKs covered

All six SDKs have generated bindings for SessionHistoryClearContextParams/Result, SessionHistoryApi.clearContext, and the session.context_cleared event. These come from main's codegen against @github/copilot@1.0.78. Confirmed present in all generated directories.

Tests

Each SDK has new tests covering both the set and omit (default) cases. Java additionally guards the older-arity constructor compatibility.

No cross-SDK consistency issues found. The PR is comprehensive and well-aligned across all languages.

Generated by SDK Consistency Review Agent for #2129 · sonnet46 52.6 AIC · ⌖ 8.33 AIC · ⊞ 6.6K ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file waiting-for-runtime-update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants